From aa17b61e5fc68c14b155a8a3841babc6bd1265bf Mon Sep 17 00:00:00 2001 From: Evgen Druzhynin Date: Fri, 9 Jun 2017 13:14:19 +0300 Subject: [PATCH] Revert "Credentials for multiple hosts." This reverts commit 29960d2528e65208dacef21252556a2415f87ad7. --- src/bin/login.rs | 22 +++--- src/cargo/core/source.rs | 2 +- src/cargo/ops/registry.rs | 60 ++++------------ src/cargo/util/config.rs | 113 ++++++++++-------------------- tests/login.rs | 143 -------------------------------------- tests/registry.rs | 12 ++-- 6 files changed, 62 insertions(+), 290 deletions(-) delete mode 100644 tests/login.rs diff --git a/src/bin/login.rs b/src/bin/login.rs index 95d0052bf..6e7841717 100644 --- a/src/bin/login.rs +++ b/src/bin/login.rs @@ -40,30 +40,26 @@ pub fn execute(options: Options, config: &Config) -> CliResult { &options.flag_color, options.flag_frozen, options.flag_locked)?; - - let host = match options.flag_host { - Some(host) => host, + let token = match options.arg_token.clone() { + Some(token) => token, None => { let src = SourceId::crates_io(config)?; let mut src = RegistrySource::remote(&src, config); src.update()?; - src.config()?.unwrap().api - } - }; - - let token = match options.arg_token { - Some(token) => token, - None => { - println!("please visit {}me and paste the API Token below", &host); + let config = src.config()?.unwrap(); + let host = options.flag_host.clone().unwrap_or(config.api); + println!("please visit {}me and paste the API Token below", host); let mut line = String::new(); let input = io::stdin(); input.lock().read_line(&mut line).chain_err(|| { "failed to read stdin" })?; - line.trim().to_string() + line } }; - ops::registry_login(config, token, host)?; + let token = token.trim().to_string(); + ops::registry_login(config, token)?; Ok(()) } + diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs index f0073fe09..9f32d692d 100644 --- a/src/cargo/core/source.rs +++ b/src/cargo/core/source.rs @@ -232,7 +232,7 @@ impl SourceId { /// This is the main cargo registry by default, but it can be overridden in /// a `.cargo/config`. pub fn crates_io(config: &Config) -> CargoResult { - let cfg = ops::registry_configuration(config, "https://crates.io")?; + let cfg = ops::registry_configuration(config)?; let url = if let Some(ref index) = cfg.index { static WARNED: AtomicBool = ATOMIC_BOOL_INIT; if !WARNED.swap(true, SeqCst) { diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 5b457e48c..78681a690 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -177,39 +177,25 @@ fn transmit(config: &Config, } } -pub fn registry_configuration(config: &Config, - host: &str) -> CargoResult { - let mut index = None; - let mut token = None; - - if !host.is_empty() { - index = config.get_string(&format!("registry.{}.index", host))?; - token = config.get_string(&format!("registry.{}.token", host))?; - } - - // FIXME: Checking out for the values which were picked up from - // $CARGO_HOME/config. This section should be removed after all the users - // start to use $CARGO_HOME/credentials for token configuration. - if index.is_none() && token.is_none() { - index = config.get_string("registry.index")?; - token = config.get_string("registry.token")?; - } - - Ok(RegistryConfig { - index: index.map(|p| p.val), - token: token.map(|p| p.val) - }) +pub fn registry_configuration(config: &Config) -> CargoResult { + let index = config.get_string("registry.index")?.map(|p| p.val); + let token = config.get_string("registry.token")?.map(|p| p.val); + Ok(RegistryConfig { index: index, token: token }) } pub fn registry(config: &Config, token: Option, index: Option) -> CargoResult<(Registry, SourceId)> { // Parse all configuration options + let RegistryConfig { + token: token_config, + index: _index_config, + } = registry_configuration(config)?; + let token = token.or(token_config); let sid = match index { Some(index) => SourceId::for_registry(&index.to_url()?), None => SourceId::crates_io(config)?, }; - let api_host = { let mut src = RegistrySource::remote(&sid, config); src.update().chain_err(|| { @@ -217,12 +203,6 @@ pub fn registry(config: &Config, })?; (src.config()?).unwrap().api }; - - let RegistryConfig { - token: token_config, - index: _index_config, - } = registry_configuration(config, &api_host)?; - let token = token.or(token_config); let handle = http_handle(config)?; Ok((Registry::new_handle(api_host, token, handle), sid)) } @@ -301,31 +281,15 @@ pub fn http_timeout(config: &Config) -> CargoResult> { Ok(env::var("HTTP_TIMEOUT").ok().and_then(|s| s.parse().ok())) } -pub fn registry_login(config: &Config, - token: String, - host: String) -> CargoResult<()> { - let host = match host.to_url()?.host_str() { - Some(h) => h.to_string(), - None => host, - }; - let host: String = host.chars() - .map(|x| match x { - '\\'|'/'|':'|'.'|'-' => '_', - _ => x, - }).collect(); - - let RegistryConfig { - index: _, - token: old_token - } = registry_configuration(config, &host)?; - +pub fn registry_login(config: &Config, token: String) -> CargoResult<()> { + let RegistryConfig { index: _, token: old_token } = registry_configuration(config)?; if let Some(old_token) = old_token { if old_token == token { return Ok(()); } } - config::save_credentials(config, token, host) + config::save_credentials(config, token) } pub struct OwnersOptions { diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index a4e94b085..b99285ce7 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -5,7 +5,6 @@ use std::collections::HashSet; use std::env; use std::fmt; use std::fs::{self, File}; -use std::io::SeekFrom; use std::io::prelude::*; use std::mem; use std::path::{Path, PathBuf}; @@ -430,52 +429,28 @@ impl Config { Ok(()) }).chain_err(|| "Couldn't load Cargo configuration")?; - self.load_credentials(&mut cfg)?; - match cfg { - CV::Table(map, _) => Ok(map), + let mut map = match cfg { + CV::Table(map, _) => map, _ => unreachable!(), - } - } + }; - fn load_credentials(&self, cfg: &mut ConfigValue) -> CargoResult<()> { let home_path = self.home_path.clone().into_path_unlocked(); - let credentials = home_path.join("credentials"); - if !fs::metadata(&credentials).is_ok() { - return Ok(()); + let token = load_credentials(&home_path)?; + if let Some(t) = token { + if !t.is_empty() { + let mut registry = map.entry("registry".into()) + .or_insert(CV::Table(HashMap::new(), PathBuf::from("."))); + match *registry { + CV::Table(ref mut m, _) => { + m.insert("token".into(), + CV::String(t, home_path.join("credentials"))); + } + _ => unreachable!(), + } + } } - let mut contents = String::new(); - let mut file = File::open(&credentials)?; - file.read_to_string(&mut contents).chain_err(|| { - format!("failed to read configuration file `{}`", - credentials.display()) - })?; - - let toml = cargo_toml::parse(&contents, - &credentials, - self).chain_err(|| { - format!("could not parse TOML configuration in `{}`", - credentials.display()) - })?; - let value = CV::from_toml(&credentials, toml).chain_err(|| { - format!("failed to load TOML configuration from `{}`", - credentials.display()) - })?; - - let mut cfg = match *cfg { - CV::Table(ref mut map, _) => map, - _ => unreachable!(), - }; - - let mut registry = cfg.entry("registry".into()) - .or_insert(CV::Table(HashMap::new(), - PathBuf::from("."))); - registry.merge(value).chain_err(|| { - format!("failed to merge configuration at `{}`", - credentials.display()) - })?; - - Ok(()) + Ok(map) } /// Look for a path for `tool` in an environment variable or config path, but return `None` @@ -592,21 +567,6 @@ impl ConfigValue { } } - fn into_toml(self) -> toml::Value { - match self { - CV::Boolean(s, _) => toml::Value::Boolean(s), - CV::String(s, _) => toml::Value::String(s), - CV::Integer(i, _) => toml::Value::Integer(i), - CV::List(l, _) => toml::Value::Array(l - .into_iter() - .map(|(s, _)| toml::Value::String(s)) - .collect()), - CV::Table(l, _) => toml::Value::Table(l.into_iter() - .map(|(k, v)| (k, v.into_toml())) - .collect()), - } - } - fn merge(&mut self, from: ConfigValue) -> CargoResult<()> { match (self, from) { (&mut CV::String(..), CV::String(..)) | @@ -807,32 +767,15 @@ fn walk_tree(pwd: &Path, mut walk: F) -> CargoResult<()> } pub fn save_credentials(cfg: &Config, - token: String, - host: String) -> CargoResult<()> { + token: String) -> CargoResult<()> { let mut file = { cfg.home_path.create_dir()?; cfg.home_path.open_rw(Path::new("credentials"), cfg, "credentials' config file")? }; - let mut map = HashMap::new(); - map.insert("token".to_string(), - ConfigValue::String(token, file.path().to_path_buf())); - - let mut contents = String::new(); - file.read_to_string(&mut contents).chain_err(|| { - format!("failed to read configuration file `{}`", - file.path().display()) - })?; - let mut toml = cargo_toml::parse(&contents, file.path(), cfg)?; - toml.as_table_mut() - .unwrap() - .insert(host, CV::Table(map, file.path().to_path_buf()).into_toml()); - - let contents = toml.to_string(); - file.seek(SeekFrom::Start(0))?; - file.write_all(contents.as_bytes())?; - file.file().set_len(contents.len() as u64)?; + file.write_all(token.as_bytes())?; + file.file().set_len(token.len() as u64)?; set_permissions(file.file(), 0o600)?; return Ok(()); @@ -853,3 +796,19 @@ pub fn save_credentials(cfg: &Config, Ok(()) } } + +fn load_credentials(home: &PathBuf) -> CargoResult> { + let credentials = home.join("credentials"); + if !fs::metadata(&credentials).is_ok() { + return Ok(None); + } + + let mut token = String::new(); + let mut file = File::open(&credentials)?; + file.read_to_string(&mut token).chain_err(|| { + format!("failed to read configuration file `{}`", + credentials.display()) + })?; + + Ok(Some(token.trim().into())) +} diff --git a/tests/login.rs b/tests/login.rs deleted file mode 100644 index c0467d663..000000000 --- a/tests/login.rs +++ /dev/null @@ -1,143 +0,0 @@ -#[macro_use] -extern crate cargotest; -extern crate cargo; -extern crate hamcrest; -extern crate toml; - -use std::io::prelude::*; -use std::fs::{self, File}; - -use cargo::util::to_url::ToUrl; -use cargotest::cargo_process; -use cargotest::support::execs; -use cargotest::support::registry::registry; -use cargotest::install::cargo_home; -use hamcrest::{assert_that, existing_file, is_not}; - -const TOKEN: &str = "test-token"; -const CONFIG_FILE: &str = r#" - [registry] - token = "api-token" -"#; - -fn setup_old_credentials() { - let config = cargo_home().join("config"); - t!(fs::create_dir_all(config.parent().unwrap())); - t!(t!(File::create(&config)).write_all(&CONFIG_FILE.as_bytes())); -} - -fn setup_new_credentials() { - let config = cargo_home().join("credentials"); - t!(fs::create_dir_all(config.parent().unwrap())); - t!(t!(File::create(&config)).write_all(format!(r#" - [crates_io] - token = "api-token" - - [{key}] - token = "api-token" - "#, key = host_to_key(registry().to_string())) - .as_bytes())); -} - -fn host_to_key(host: String) -> String { - let host = match host.to_url().unwrap().host_str() { - Some(h) => h.to_string(), - None => host, - }; - - host.chars() - .map(|x| match x { - '\\'|'/'|':'|'.'|'-' => '_', - _ => x, - }).collect() -} - -fn check_host_token(mut toml: toml::Value, host_key: &str) -> bool { - for &key in [host_key, "token"].into_iter() { - match toml { - toml::Value::Table(table) => { - if let Some(v) = table.get(key) { - toml = v.clone(); - } else { - return false; - } - } - _ => break, - } - } - - match toml { - toml::Value::String(token) => (&token == TOKEN), - _ => false, - } -} - -#[test] -fn login_with_old_credentials() { - setup_old_credentials(); - - assert_that(cargo_process().arg("login") - .arg("--host").arg(registry().to_string()).arg(TOKEN), - execs().with_status(0)); - - let config = cargo_home().join("config"); - assert_that(&config, existing_file()); - - let mut contents = String::new(); - File::open(&config).unwrap().read_to_string(&mut contents).unwrap(); - assert!(CONFIG_FILE == &contents); - - let credentials = cargo_home().join("credentials"); - assert_that(&credentials, existing_file()); - - contents.clear(); - File::open(&credentials).unwrap().read_to_string(&mut contents).unwrap(); - assert!(check_host_token(contents.parse().unwrap(), - &host_to_key(registry().to_string()))); -} - -#[test] -fn login_with_new_credentials() { - setup_new_credentials(); - - assert_that(cargo_process().arg("login") - .arg("--host").arg(registry().to_string()).arg(TOKEN), - execs().with_status(0)); - - let config = cargo_home().join("config"); - assert_that(&config, is_not(existing_file())); - - let credentials = cargo_home().join("credentials"); - assert_that(&credentials, existing_file()); - - let mut contents = String::new(); - File::open(&credentials).unwrap().read_to_string(&mut contents).unwrap(); - assert!(check_host_token(contents.parse().unwrap(), - &host_to_key(registry().to_string()))); -} - -#[test] -fn login_with_old_and_new_credentials() { - setup_new_credentials(); - login_with_old_credentials(); -} - -#[test] -fn login_without_credentials() { - assert_that(cargo_process().arg("login") - .arg("--host").arg(registry().to_string()).arg(TOKEN), - execs().with_status(0)); - - let config = cargo_home().join("config"); - assert_that(&config, is_not(existing_file())); - - let credentials = cargo_home().join("credentials"); - assert_that(&credentials, existing_file()); - - let mut contents = String::new(); - File::open(&credentials).unwrap().read_to_string(&mut contents).unwrap(); - let toml: toml::Value = contents.parse().unwrap(); - - assert!(check_host_token(toml.clone(), - &host_to_key(registry().to_string()))); -} diff --git a/tests/registry.rs b/tests/registry.rs index 5983cdd95..b336827ab 100644 --- a/tests/registry.rs +++ b/tests/registry.rs @@ -581,8 +581,7 @@ fn dev_dependency_not_used() { fn login_with_no_cargo_dir() { let home = paths::home().join("new-home"); t!(fs::create_dir(&home)); - assert_that(cargo_process().arg("login").arg("--host").arg(registry().to_string()) - .arg("foo").arg("-v"), + assert_that(cargo_process().arg("login").arg("foo").arg("-v"), execs().with_status(0)); } @@ -591,14 +590,11 @@ fn login_with_differently_sized_token() { // Verify that the configuration file gets properly trunchated. let home = paths::home().join("new-home"); t!(fs::create_dir(&home)); - assert_that(cargo_process().arg("login").arg("--host").arg(registry().to_string()) - .arg("lmaolmaolmao").arg("-v"), + assert_that(cargo_process().arg("login").arg("lmaolmaolmao").arg("-v"), execs().with_status(0)); - assert_that(cargo_process().arg("login").arg("--host").arg(registry().to_string()) - .arg("lmao").arg("-v"), + assert_that(cargo_process().arg("login").arg("lmao").arg("-v"), execs().with_status(0)); - assert_that(cargo_process().arg("login").arg("--host").arg(registry().to_string()) - .arg("lmaolmaolmao").arg("-v"), + assert_that(cargo_process().arg("login").arg("lmaolmaolmao").arg("-v"), execs().with_status(0)); } -- 2.30.2